fix(webapp): stop double-fetching custom registries in advanced mode#2721
fix(webapp): stop double-fetching custom registries in advanced mode#2721thedavidmeister wants to merge 8 commits into
Conversation
loadRegistryUrl() called DotrainRegistry.validate(url) to validate a custom
registry before persisting it and reloading the page. The post-reload
+layout.ts then calls DotrainRegistry.new(url), which fetches and validates the
exact same registry file plus all its settings files from GitHub again. Every
custom-registry load therefore downloaded the rain documents twice, which
trips GitHub's rate limiter ("Too many requests").
Remove the redundant validate() call from loadRegistryUrl(); it now just
persists the URL and reloads. Validation is owned by page-load: +layout.ts
already surfaces a DotrainRegistry.new() failure as an errorMessage on the
error page, so an invalid registry URL is still reported to the user, just
after the reload instead of before, without the duplicate fetch.
Tests: rewrite loadRegistryUrl.test.ts to assert the function calls neither
DotrainRegistry.validate nor DotrainRegistry.new (the regression guard), that
it persists then reloads in that order, and that it never reloads when
setRegistry throws. Confirmed the old (validate) implementation fails these.
Fixes #2046
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough
ChangesloadRegistryUrl simplification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… comments Rewrite the comments added in this PR so they describe current behavior only: loadRegistryUrl persists the URL and reloads, and fetching/validation is owned by the post-reload +layout.ts via DotrainRegistry.new. No code/logic change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he app When DotrainRegistry.new fails for a custom registry (from the ?registry= param or localStorage), +layout.ts clears the persisted custom registry and retries the default registry so the app still mounts, surfacing a non-fatal warning banner. A failing default registry remains a fatal ErrorPage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
screenshot pending (manual): headless chromium requires a 375MB nix download; the changed markup is a warning toast in layout.svelte that appears at the bottom when registryWarning is set. |
|
Screenshot (static harness render — text not visible due to headless font rendering, but yellow toast element is confirmed positioned at bottom-center):
The |
1 participant
Fixes #2046
Problem
In advanced mode, loading a custom registry fetched the rain documents from GitHub twice, which triggers GitHub's rate limiter ("Too many requests for fetching rain documents from github").
loadRegistryUrl()(packages/ui-components/src/lib/services/loadRegistryUrl.ts) calledDotrainRegistry.validate(url)— which fetches the registry file plus all its referenced settings files — before persisting the URL and reloading. After the reload,+layout.tscallsDotrainRegistry.new(url), which fetches and validates the same files all over again. So every custom-registry load downloaded everything twice.Fix
Remove the redundant
validate()call fromloadRegistryUrl(). It now just persists the URL viaregistryManager.setRegistry(url)and reloads. Validation is owned by page-load:+layout.tsalready turns aDotrainRegistry.new()failure into anerrorMessagerendered on the error page, so an invalid registry URL is still reported to the user — just after the reload instead of before — with no duplicate fetch.Tests
Rewrote
loadRegistryUrl.test.tswith discriminating assertions:loadRegistryUrlcalls neitherDotrainRegistry.validatenorDotrainRegistry.new— it performs no registry fetch of its own. Re-introducing a fetch (the bug) fails this.setRegistryruns beforereload.setRegistrythrows and does not reload, and maps a non-Error throw to the default message.Verified the old (validate-based) implementation fails these new tests (5/7 fail), confirming they catch the regression.
Verification
npx vitest run loadRegistryUrl.test.ts InputRegistryUrl.test.ts→ 12/12 pass+layout.tstest (the page-load validation path this fix relies on) → 4/4 passnpm run lint(ui-components) → cleannpm run check(ui-components, svelte-check) → 0 errors, 0 warningsnpm run build -w @rainlanguage/ui-components→ success🤖 Generated with Claude Code
Summary by CodeRabbit